Full support for OpenTelemetry tracing, use it instead of XRay in runDB
#120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pbrisbin
changed the title
runDB
This is actually a smell, IMO, that I have to deal with this in our own clients, so I'm going to remove the re-exports and see if things work out better. It should mean adding |
This makes it harder for clients to decide between the default OTel tracing vs XRay-based. It was always a bit too opinionated anyway. Having to bring in the Database module only if/when you need it is kind of nice anyway.
chris-martin
approved these changes
Sep 23, 2023
|
OK, I tossed in a bunch of packaging fixes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was spiked, implemented, used in platform-example-app, then shelved
because megarepo wasn't ready for it. I got nervous having the code live in
a branch and devised a way to land it in
mainso we could use it from PlatformApps, but still be able to keep megarepo on XRay.
The CHANGELOG sums up well where we landed:
This replaces our bespoke, XRay-based
MonadTracerclass to be that ofhs-opentelemetry. This means
Freckle.App.Database.runDBnow traces usingOpenTelemetry instead of XRay. A compilation error will indicate a missing
HasTracerinstance, which you should implement based on theFreckle.App.OpenTelemetrymodule documentation.Clients who wish to remain on XRay can still upgrade, but must adjust their
imports so they use
Freckle.App.Database.XRayinstead. TheFreckle.App.Databasemodule is re-exported throughFreckle.AppandFreckle.App.Test, so there may be some additionalhidingrequired. Acompilation error indicating a missing
HasTracerorMonadTracerinstancemeans your imports are not yet correct.
I've already tested using this from megarepo (staying on XRay) and progress
(opting in for tracing), so I'm relatively confident the
.XRaymodule plan works.